Skip to content

Conversation

@ypatil12
Copy link
Collaborator

@ypatil12 ypatil12 commented Jan 5, 2026

Motivation:

Address L-01 finding from the Certora audit: "Instant slasher setting leaves stale slasher field in storage". When _updateSlasher() is called with instantEffectBlock=true, only pendingSlasher and effectBlock are updated, leaving the slasher field stale (address(0) for new operator sets). While getSlasher() returns correct values, raw storage is inconsistent.

Modifications:

  • Updated _updateSlasher() in AllocationManager.sol to set params.slasher = slasher when instantEffectBlock=true
  • Updated NatSpec for SlasherParams struct in IAllocationManager.sol to document storage behavior
  • Added getSlasherParams() helper to AllocationManagerHarness.sol for testing raw storage
  • Added unit tests for instant slasher storage consistency
  • Updated documentation in docs/core/AllocationManager.md

Result:

  • Storage is now consistent when creating operator sets or migrating slashers
  • Both slasher and pendingSlasher fields are set immediately when instantEffectBlock=true
  • Raw storage reads will return expected values matching getSlasher() return value

🤖 Generated with Claude Code

Addresses audit finding L-01: Instant slasher setting leaves stale slasher field in storage.

When a slasher is set with immediate effect (via createOperatorSets or migrateSlashers),
_updateSlasher now also updates the params.slasher field, not just pendingSlasher.
This ensures storage consistency where the raw SlasherParams struct reflects the
effective slasher immediately.

Changes:
- AllocationManager._updateSlasher(): Set params.slasher when instantEffectBlock=true
- IAllocationManager: Updated NatSpec for SlasherParams struct
- AllocationManagerHarness: Added getSlasherParams() for testing raw storage
- Unit tests: Added tests verifying storage consistency
- Docs: Updated createOperatorSets and migrateSlashers effects

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@ypatil12 ypatil12 force-pushed the feat/slashing-improvements-audit-fixes branch from cbb859e to e4bf888 Compare January 5, 2026 02:13
Copy link
Member

@0xClandestine 0xClandestine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@ypatil12 ypatil12 merged commit e9bb4b0 into feat/slashing-improvements-audit-fixes Jan 8, 2026
9 checks passed
@ypatil12 ypatil12 deleted the fix/L01-slasher-storage branch January 8, 2026 16:17
ypatil12 added a commit that referenced this pull request Jan 8, 2026
…1687)

**Motivation:**

Address L-01 finding from the Certora audit: "Instant slasher setting
leaves stale slasher field in storage". When `_updateSlasher()` is
called with `instantEffectBlock=true`, only `pendingSlasher` and
`effectBlock` are updated, leaving the `slasher` field stale (address(0)
for new operator sets). While `getSlasher()` returns correct values, raw
storage is inconsistent.

**Modifications:**

- Updated `_updateSlasher()` in `AllocationManager.sol` to set
`params.slasher = slasher` when `instantEffectBlock=true`
- Updated NatSpec for `SlasherParams` struct in `IAllocationManager.sol`
to document storage behavior
- Added `getSlasherParams()` helper to `AllocationManagerHarness.sol`
for testing raw storage
- Added unit tests for instant slasher storage consistency
- Updated documentation in `docs/core/AllocationManager.md`

**Result:**

- Storage is now consistent when creating operator sets or migrating
slashers
- Both `slasher` and `pendingSlasher` fields are set immediately when
`instantEffectBlock=true`
- Raw storage reads will return expected values matching `getSlasher()`
return value

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
ypatil12 added a commit that referenced this pull request Jan 13, 2026
**Motivation:**

Address findings from the Certora audit of EigenLayer Slashing UX
Improvements. This branch consolidates fixes for low-severity findings
(L-01) and informational findings (I-01 through I-05) across
AllocationManager and ProtocolRegistry contracts.

**Modifications:**

### AllocationManager Fixes

**L-01: State inconsistency fixes**
- Update `slasher` field immediately when `instantEffectBlock=true` to
prevent stale storage (#1687)
- Update `delay` and `isSet` fields immediately for newly registered
operators to ensure storage consistency (#1688)

**I-01: Re-proposing same pending slasher is now a no-op** (#1689)
- Added check in `_updateSlasher()` to skip processing if the proposed
slasher is already pending
- Prevents accidentally restarting the delay countdown

**I-02: Add NatSpec documentation for getSlasher/getPendingSlasher**
(#1689)
- Document that these functions return `address(0)`/`0` for non-existent
operator sets

**I-03: Add separate SLASHER_CONFIGURATION_DELAY constant** (#1689)
- New immutable allows independent configuration of slasher delay in
future upgrades
- Currently set to same value as `ALLOCATION_CONFIGURATION_DELAY`

**I-05: Add gas warning documentation for migrateSlashers** (#1689)
- Document O(appointees) gas cost per operator set
- Warn about potential block gas limit issues with large appointee sets

### ProtocolRegistry Fixes

**I-01: ship() lacks validation** (#1690)
- Added array length validation for addresses, configs, and names
- Added zero address validation with new `ArrayLengthMismatch()` and
`InputAddressZero()` errors

**I-02: Orphaned configs on name overwrite** (#1690)
- Delete old address's `DeploymentConfig` when re-shipping a name with a
new address
- Added `DeploymentConfigDeleted(address)` event

**I-03: configure() for unshipped addresses** (#1690)
- Updated `configure` to require a name parameter
- Added validation that address must be a shipped deployment

**I-04: Fix misleading NatSpec** (#1690)
- Clarified `ship()` behavior when re-shipping names
- Updated `configure()` NatSpec for address requirements

**I-05: Document pauseAll blocking** (#1690)
- Added warning that `pauseAll()` reverts if ANY pausable deployment
fails

### Other Changes
- v1.9.0 upgrade script fixes for testnet deploy (#1677)
- Added Claude skills for development workflow (#1686)

**Result:**

- Audit fixes complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants